Backport: eager connect watchdog for InPlay/iPhone-16 wedges - #141
Conversation
Backport of #133 from next-dev, squashed. Mitigates BLE connection wedges on InPlay-firmware DASH pods paired with an iPhone 16 (any variant) or 17e: a connect that stalls with no callback is torn down and retried rather than left to time out, and the pod settings view gains a persistent notice explaining the slower connections. Also carries the later fixes from that branch: the O5 fresh-advert connect, which cut foreground connects on Omnipod 5 from about six seconds to under one; the foreground-state seed, so a pump manager built after the app has already become active does not idle-disconnect between commands; and the test-beeps button no longer gating on hasConnection. Two conflicts against this line, both because dev predates the Pod Keep Alive rework (#125). Resolved by keeping dev's accessor and taking the eager-connect logic: - shouldHoldConnection keeps podType.isDash && Storage.shared.podKeepAlive.value.keepsPodConnectedInBackground rather than podKeepAliveKeepsConnectedInBackground - the pairing auto-enable keeps Storage.shared.podKeepAlive.value while adopting the consolidated UIDevice.hasPossibleInPlayBLEIssues Does not build against LoopWorkspace/dev as it stands, for reasons that predate this change -- see the pull request.
5df3bb2 to
5a7350a
Compare
|
Rebased onto current Conflicts, and how they were resolvedBoth were the same shape: #142 introduced newer accessors on 1.
if isAppForeground { return true }
if let peripheral = keepAlivePeripheral, shouldUseEagerConnect(for: peripheral) { return true }
return podKeepAliveKeepsConnectedInBackgroundByte-identical to 2. A genuine split — took each side's newer half, which is again what if self.usingInPlayPod == true && UIDevice.hasPossibleInPlayBLEIssues { // ours (consolidated helper)
if self.state.podKeepAlive == .disabled { // dev's, from #142
The original blocker is goneThis PR was opened noting it couldn't build until Build
That required one unrelated fix: Still a draft: hardware testing on an InPlay pod + iPhone 16/17e is the remaining gap. |
itsmojo
left a comment
itsmojo
left a comment
There was a problem hiding this comment.
Merge looks good and basic functionality checks out on a non-iPhone 16/17e. Unfortunately I am unable to perform any real testing of the new eager connect functionality without having an iPhone that exhibits the problematic behavior with an InPlay BLE DASH pod.
setBLEHeartbeatRequest branched on podType.mayUseRileyLink, which is true for DASH as well as Eros because DASH can use a RileyLink under the Pod Keep Alive option. A DASH pod on direct BLE with Pod Keep Alive off therefore took the RileyLink branch and only set timerTickEnabled on a RileyLink provider that has no devices. provideHeartbeat was never assigned and BlePodComms.setHeartbeatRequest was never called, so the delayed-connect probe that produces periodic background wakes was never scheduled. issueHeartbeatIfNeeded() also gates on provideHeartbeat, so even a probe that did fire would not have notified Loop. Loop was requesting the heartbeat correctly -- DeviceDataManager sets pumpManagerMustProvideBLEHeartbeat when the CGM cannot provide one -- and the request was logged and then dropped. With no wake source the app stayed suspended in the background until something else woke it. Seen as Loop Failure alerts after gaps of 20 to 264 minutes that end the instant the app is foregrounded; a report from an affected DASH pod shows provideHeartbeat: false moments after a setBLEHeartbeatRequest log line. Only reachable when the CGM cannot supply the heartbeat itself, so it needs a remote or networked CGM such as Nightscout. A BLE Dexcom provides its own heartbeat and masks it. Branch on whether a RileyLink is actually in use instead: Eros always, DASH only under the RileyLink Pod Keep Alive option. The other mayUseRileyLink call sites want the "may" semantics and are unchanged.
|
Added a second commit (0f3295f) for a separate background-wake bug found while testing this branch on a DASH pod with Nightscout Remote CGM.
Loop was asking correctly: It needs a CGM that cannot supply its own heartbeat, so remote/networked CGMs like Nightscout — a BLE Dexcom masks it entirely. That is presumably why it has gone unreported. The fix branches on whether a RileyLink is actually in use (Eros always; DASH only under the RileyLink Pod Keep Alive option) rather than whether one is possible. I checked the other six This is orthogonal to the watchdog, so happy to split it into its own PR against Separately, one thing in this branch worth a look. The new |
2 participants
Backport of #133 to the
devline, squashed to a single commit.What it brings
Mitigation for BLE connection wedges on InPlay-firmware DASH pods paired with an iPhone 16 (any variant) or 17e: a connect that stalls with no callback is torn down and retried rather than left to time out, plus a persistent notice in pod settings explaining the slower connections and a detail screen behind it.
It also carries the later fixes from that branch:
didBecomeActive, soisAppForegroundstayed false and the link was idle-disconnected ~4s after each command while the user was on screen.hasConnection.Conflicts
Rebased onto
devat2c603f7. Two conflicts, both with #142 (the Pod Keep Alive backport), which introduced newer accessors that this branch predated.Neither resolution is a judgment call — each reproduces exactly what
next-devalready carries, that being the lineage where both changes coexist.1.
BluetoothManager.shouldHoldConnection— kept dev'spodKeepAliveKeepsConnectedInBackgroundaccessor and layered the eager-connect early return above it:Byte-identical to
next-dev.2.
OmniPumpManagerpairing auto-enable — a genuine split; took each side's newer half, which is again whatnext-devhas:Storage.shared.podKeepAlive.valueno longer appears anywhere in the tree. The backport removes dev's localOmniPumpManager.iPhoneWithPossibleInPlayIssuesand shipsOmnipodKit/Common/UIDevice.swiftwith the consolidated helper, so there is no leftover duplicate — zero references remain to the old name.Verification
** BUILD SUCCEEDED **, exit 0 — LoopWorkspace scheme, iPhone 17 simulator, LoopWorkspacedevwith this branch checked out for OmnipodKit.One unrelated fix was required to get there:
devstill has theusesRileyLinkbreak that #146 fixed onmain, soOmnipodKit/devcannot currently build for any simulator destination, with or without this PR. Raised separately as #148. With that applied, this branch builds clean — it was the only failure.Not exercised on hardware. Testing on an InPlay pod with an iPhone 16 or 17e is the remaining gap, which is why this stays a draft.
Note on the earlier "does not build against LoopWorkspace/dev" section
That section is obsolete and has been removed. It described
OmnipodKit/devrequiringPumpHeartbeatRequest, a LoopKit type the pinned LoopKit did not define. That has since landed via LoopKit #599 —LoopWorkspace/devnow pins LoopKita5beee96, which defines it. The dependency is satisfied and this PR is no longer blocked on a LoopKit bump.